Stabilize ADX, TBM, and SSE4a target features #60109
Conversation
rust-highfive
added
the
S-waiting-on-review
|
The ui tests will probably need updating. Also, I'm not sure if Rust 1.35.0 is the right version to use here. |
This comment has been minimized.
This comment has been minimized.
Centril
added
relnotes
|
@gnzlbg Looks good, tho the docs of https://doc.rust-lang.org/nightly/core/arch/x86_64/fn._mm_stream_ss.html could be a bit more illuminating for the unfamiliar but curious reader. MSDN provides more elaboration: https://docs.microsoft.com/en-us/cpp/intrinsics/mm-stream-ss?view=vs-2019. An idea for the future: Can we feasibly link to the relevant vendor documentation for each function, e.g. Intel's docs for a specific intrinsic? Also, the current nightly is 1.36 so 1.35 seems wrong? @alexcrichton shall we start FCP? |
|
@rfcbot fcp merge |
|
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
Not really. Intel provides two types of documentation: the intrinsics guide, which is often not very informative, and the software developer's manual, which is a many-1000 page PDF document, so it's not easy to link to. |
|
Agree with @jethrogb . In this case, SSE4a is not an Intel intrinsic, but an AMD one, and AMD does the exact same thing (1000s of pages long pdfs). These pdfs are not designed with tools that want to parse them in mind, so I don't think it's a feasible thing to do. The Intel Intrinsics guide is available in XML format, and we do parse that for automatic verification of the intrinsics names/type signatures/instructions/etc. but the docs in there are not very good. The clang docs are better, and the MSVC docs are even better (they often contain working examples for each intrinsic), so maybe we could automatically parse those and use them to generate documentation (at least under the assumption that we could sort out licensing issues). If somebody wants to work on doing this I'll be very happy to mentor. |
I'll fix the tests and set this to 1.36.0! |
|
☔ The latest upstream changes (presumably #60416) made this pull request unmergeable. Please resolve the merge conflicts. |
|
NB: I checked @aturon's box as he is presently on leave. |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
added
the
finished-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
rfcbot
removed
the
final-comment-period
|
@gnzlbg want to rebase and I'll r+? |
Centril
added
S-waiting-on-author
|
When you rebase, remember to adjust the stabilization dates to 1.37.0. |
gnzlbg
force-pushed
the
stabilize_target_features
branch
from
f6059b1
to
672d0e6
Compare
|
This has slipped to 1.38 and there is a conflict. |
|
Ping from triage @gnzlbg any updates on this? |
|
Ping from triage: @gnzlbg, you should resolve conflict and bump up the target version to 1.38. |
|
Ping from triage: @gnzlbg, you should resolve conflicts and bump up the target version to 1.39. |
|
Ping from triage @gnzlbg, thank you for your work on this, please re-open this PR once you can resolve the conflicts (please re-open first then push). |
hdhoang
added
S-inactive
|
@gnzlbg Are you still planning to finish this work? |
This is a continuation of rust-lang#60109, which noted that while the ADX intrinsics were stabilized, the corresponding target feature never was. This PR follows the same general structure and stabilizes the ADX target feature.
Stabilize ADX target feature This is a continuation of rust-lang#60109, which noted that while the ADX intrinsics were stabilized, the corresponding target feature never was. This PR follows the same general structure and stabilizes the ADX target feature. See also rust-lang#44839 - tracking issue for target feature
This PR stabilizes the ADX, TBM, and SSE4a target features.
We stabilized the ADX
core::archintrinsics in Rust 1.33, and the TBM and SSE4a intrinsics have been accidentally stabilized since Rust 1.27. AFAICT there is no good reason to leave those as stabilized (we got lucky I guess), and while we could unstabilize them, it seems that they are worth stabilizing right after, so that would just introduce unnecessary churn.It seems that when we stabilized the ADX intrinsics, we just forgot to also stabilize the ADX target feature attribute.
r? @alexcrichton
cc @jethrogb @Centril @rust-lang/libs (for the decision of not unstabilizing the accidentally stabilized intrinsics) @rust-lang/lang (for the decision of stabilizing the
#[target_feature(enable = "adx,sse4a,tbm")]target features).cc #44839 - tracking issue for target feature